[Commuter Mileage Exclusion] [R1] Display and enforce fixed distance exclusions#93848
Open
fedirjh wants to merge 89 commits into
Open
[Commuter Mileage Exclusion] [R1] Display and enforce fixed distance exclusions#93848fedirjh wants to merge 89 commits into
fedirjh wants to merge 89 commits into
Conversation
Foundation for the commuter exclusion feature: - TransactionCustomUnit gains commuterExclusion, reimbursableDistance, commuterExclusionType, and commuterExclusionMethod so the App can carry the PHP-computed exclusion metadata through Onyx. - Policy gains the commuterExclusions config object (method, fixedDistance, fixedDistanceUnit) so feature gating and the optimistic client-side compute can read it. - CONST.COMMUTER_EXCLUSIONS namespaces the method/type enums; a new COMMUTER_EXCLUSION report action type and a POLICYCHANGELOG_UPDATE_COMMUTER_EXCLUSIONS policy-change-log type are registered for the system message and workspace audit log. - en.ts adds the three-line breakdown labels, the manual/odometer blocked error copy, and the system-message fallback.
Add a DistanceWithCommuterExclusion component that reads
comment.customUnit.{quantity,commuterExclusion,reimbursableDistance,
distanceUnit} and renders a three-line breakdown (original distance,
removed commute distance, reimbursable distance). The component
renders nothing when commuterExclusion is not set or non-positive.
Mount it next to the existing DistanceField in
TransactionDetailsFields (the confirmation / IOURequestStepConfirmation
surface) and after the rate row in MoneyRequestView (the expense
detail / history surface), so both creation and post-creation views
show the same breakdown.
- DistanceRequestStartPage hides the Manual and Odometer tabs when every workspace the user belongs to has commuterExclusions configured. If at least one workspace lacks exclusions, all four tabs stay visible (the user can still pick that workspace). - useConfirmationValidation: allow zero-amount submission when the transaction carries a positive commuterExclusion, so a fully excluded distance expense (reimbursable = 0) can still be submitted in the split-bill-edit branch. The existing distance-request guard already handles the create branch.
- OriginalMessage gains OriginalMessageCommuterExclusion ({distance,
unit}) keyed under CONST.REPORT.ACTIONS.TYPE.COMMUTER_EXCLUSION so
callers reading the original message are properly typed.
- ReportActionsUtils.getCommuterExclusionMessage formats the system
message via the existing distance.commuterExclusion.systemMessage
i18n key.
- ActionContentRouter renders the COMMUTER_EXCLUSION action as a
ReportActionItemBasicMessage, matching the existing pattern used
for plaid/company-card system messages.
The COMMUTER_EXCLUSION action type is already in
CONST.REPORT.ACTIONS.TYPE and therefore in the supportedActionTypes
set built dynamically in ReportActionsUtils, so it is treated as a
visible chat action without further changes.
When the destination policy has a fixedDistance commuter exclusion
configured (method === "fixedDistance" with a positive fixedDistance),
createDistanceRequest now writes:
- A merge onto the optimistic transaction that fills the four new
comment.customUnit fields (commuterExclusion, reimbursableDistance,
commuterExclusionMethod) and sets modifiedAmount / modifiedMerchant
from the reimbursable distance, so the three-line breakdown and the
reduced amount render instantly without waiting on the server.
- An optimistic COMMUTER_EXCLUSION report action posted to
reportActions_${chatReportID} (pendingAction: ADD), so the system
message renders instantly too. ActionContentRouter picks this up
and renders the localized breakdown.
Failure data reverts both side effects (transaction fields cleared,
optimistic action removed). For commuterExclusions undefined and for
the not-yet-supported homeAndOffice method, behavior is unchanged.
Adds buildOptimisticCommuterExclusionReportAction in ReportUtils
following the existing buildOptimisticCancelPaymentReportAction
pattern.
When the destination policy has commuterExclusions configured, the user is no longer allowed to submit a manual or odometer distance expense — the exclusion is computed off the mapped route, so the mapped flow is the only correct entry. Show a localized form error explaining why, instead of silently accepting an entry that the backend wouldn't apply exclusions to. This complements the tab-hiding in DistanceRequestStartPage (which only hides the tabs when *every* workspace the user belongs to has exclusions); when the user has any non-exclusion workspace, the tabs stay visible but selecting an exclusion workspace on submission surfaces this error.
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Remove the top-level COMMUTER_EXCLUSIONS object and use the existing COMMUTER_EXCLUSION_METHOD and COMMUTER_EXCLUSION_TYPE constants within CONST.POLICY instead.
Add getCommuterExclusionBreakdown to compute the excluded commuter distance and reimbursable distance (from the transaction or previewed from the policy), and getDistanceUnitLabel to return the translated, pluralized unit label.
Render the distance field as "Distance · Original: X" with the reimbursable distance as the main value and a "Removed X commuter" line via the shared DistanceWithCommuterExclusion component, on both the confirmation page and the expense view. Add the supporting translation strings.
Base the distance request amount on the reimbursable distance after the commuter exclusion, and build the optimistic transaction fields and system action on creation. Derive the quantity from the route distance when it isn't populated yet so the exclusion is reflected immediately for route-based requests.
Build the commuter exclusion and fixed-distance report action messages from a preformatted distance, and render the commuter exclusion message as HTML linking to the workspace distance settings.
…references. Update distance request handling to calculate reimbursable distance using server-side logic instead of optimistic actions.
For distance requests, always use the computed distanceRequestAmount since it accounts for commuter exclusions and rate changes. This avoids stale iouAmount values that don't reflect the current commuter exclusion.
- Return commuterExclusionBreakdown from useDistanceRequestState - Create commuterExclusionData in MoneyRequestConfirmationList with proper unit fallbacks to handle undefined cases - Update DistanceField to display commuter exclusion UI using the computed breakdown, with fallback for unit prop
When commuter exclusion applies, display the reimbursable distance (not the full distance) in the merchant text so it matches the calculated amount.
In offline mode, the transaction may not have commuterExclusion stored yet. Fall back to computing it from the policy's commuterExclusions config, enabling instant display of commuter exclusion breakdown before the server response.
Move quantity derivation into getCommuterExclusionBreakdown and let getDistanceForDisplay render the reimbursable distance via an optional commuterExclusionData param, removing duplicated formatting across MoneyRequestView and DistanceField.
…d getDistanceForDisplay
… original distance Share the display-unit formatting between getDistanceForDisplay, MoneyRequestView, and DistanceField instead of duplicating ad-hoc toFixed formatting.
Add isCommuterDistance flag and delegate the unit word to getDistanceUnitLabel, replacing duplicated commuter distance formatting in DistanceWithCommuterExclusion and ReportActionsUtils.
Updated the useDistanceRequestState hook to utilize commuterExclusionData instead of commuterExclusionBreakdown for improved clarity and consistency. Adjusted related components to reflect this change, ensuring proper data flow and display for distance requests.
Contributor
|
I can take over this as C+ based on https://expensify.slack.com/archives/C02NK2DQWUX/p1783710663647299?thread_ts=1783325340.577219&cid=C02NK2DQWUX |
heyjennahay
requested review from
JmillsExpensify
and removed request for
heyjennahay
Contributor
|
Assigned. @fedirjh, some conflicts |
Contributor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Contributor
|
I am reviewing and testing the code changes. |
Contributor
|
🚧 JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Contributor
|
@fedirjh, could you please take a look? We can still see all the tabs even after deleting the newly created workspace. RF-1.mp4 |
Contributor
|
@fedirjh, I still can't see the RF-2.mp4 |
Contributor
|
Still waiting for the beta to take effect on my account to test. |
Contributor
Author
@marufsharifi This should be posted in the transaction report, I have updated the test to mention that.
This is fixed now. |
7 tasks
Contributor
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4651d1f530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+377
to
+380
| if (typeof existingCustomUnit?.quantity === 'number') { | ||
| routeDistance = existingCustomUnit.quantity; | ||
| } else if (routeDistanceInMeters !== undefined) { | ||
| routeDistance = convertDistanceUnit(routeDistanceInMeters, requestDistanceUnit); |
There was a problem hiding this comment.
When a map/GPS route is recalculated after a commuter exclusion has already been applied, callers pass the new routeDistanceMeters while also spreading the existing customUnit, which now contains the previous quantity. This branch prefers that stale quantity, so reimbursableDistance and modifiedAmount stay based on the old route instead of the newly calculated one until the server response corrects it. Prefer the fresh route distance when it is supplied, or overwrite quantity before this calculation.
Useful? React with 👍 / 👎.
Comment on lines
+63
to
+67
| const [shouldHideManualAndOdometerTabs] = useOnyx(ONYXKEYS.COLLECTION.POLICY, { | ||
| selector: (allPolicies) => { | ||
| const activeGroupPolicies = getActivePolicies(allPolicies ?? null, login).filter(isGroupPolicy); | ||
| return activeGroupPolicies.length > 0 && activeGroupPolicies.every((activePolicy) => !!activePolicy.commuterExclusions); | ||
| }, |
There was a problem hiding this comment.
This hides the manual and odometer tabs whenever all active group policies have commuter exclusions, regardless of what this distance request is actually targeting. A user with only commuter-exclusion workspaces can still create valid personal/P2P or self-DM manual distance expenses, but those flows have policy undefined/personal and the tabs become unreachable before the later per-policy guard can make that distinction.
Useful? React with 👍 / 👎.
Contributor
|
🚧 JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
Contributor
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Implements the App-side slice of the workspace commuter exclusion feature (App issue #642599). PHP-side (Web-Expensify) lands in https://github.com/Expensify/Web-Expensify/pull/53798.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/642599
$ https://github.com/Expensify/Expensify/issues/651462
PROPOSAL:
Tests
DistanceRequestStartPage.Offline tests
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari